Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix instructions on reproducing ci errors #24402

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 12, 2022

The old breeze-legacy used to have a possibility of very easy
reproduction of CI failures by executing the right breeze command
that contained the commit hash of the PR being tested. This has
been broken for some time after we migrated to the new breeze,
but finally it was the time when it was needed again.

This PR brings back the capability by:

  • addding --image-tag parameters for tests, shell and start-airflow
    commands
  • if --image-tag is specified, then rather than building the
    image, it is pulled using the specified hash
  • if --image-tag is specified, the local sources are not mounted
    to breeze when started, but the sources already embedded in the
    image are used ("skipped" set for --mount-sources).
  • new "removed" command value is added to --mount-sources, it
    causes breeze command to remove the sources from the image (it
    is used when installing airflow during the tests for specified
    version (it's automatically used when --use-airflow-version
    is used).

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

@potiuk potiuk marked this pull request as ready for review June 12, 2022 14:36
@potiuk potiuk force-pushed the fix-instructions-on-reproducing-ci-errors branch 3 times, most recently from 61c492a to ee4a335 Compare June 13, 2022 22:11
@potiuk
Copy link
Member Author

potiuk commented Jun 13, 2022

This has been really useful when analysing #24405 - Would be great to merge #24236 and this one to make it faster to investigate other flaky tests.

@potiuk potiuk force-pushed the fix-instructions-on-reproducing-ci-errors branch from ee4a335 to 809e279 Compare June 14, 2022 14:25
The old breeze-legacy used to have a possibility of very easy
reproduction of CI failures by executing the right breeze command
that contained the commit hash of the PR being tested. This has
been broken for some time after we migrated to the new breeze,
but finally it was the time when it was needed again.

This PR brings back the capability by:

* addding --image-tag parameters for tests, shell and start-airflow
  commands
* if --image-tag is specified, then rather than building the
  image, it is pulled using the specified hash
* if --image-tag is specified, the local sources are not mounted
  to breeze when started, but the sources already embedded in the
  image are used ("skipped" set for --mount-sources).
* new "removed" command value is added to --mount-sources, it
  causes breeze command to remove the sources from the image (it
  is used when installing airflow during the tests for specified
  version (it's automatically used when --use-airflow-version
  is used).
@potiuk potiuk force-pushed the fix-instructions-on-reproducing-ci-errors branch from 809e279 to d04402a Compare June 14, 2022 21:19
@potiuk
Copy link
Member Author

potiuk commented Jun 14, 2022

It's possible now to use new breeze to reproduce failures from CI more easily :)

@potiuk
Copy link
Member Author

potiuk commented Jun 17, 2022

:) ?

@@ -173,9 +173,6 @@ function breeze_complete::get_known_values_breeze() {
-g | --github-repository)
_breeze_known_values="${_breeze_default_github_repository}"
;;
-s | --github-image-id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should github-image-id also be removed/replaced in _breeze_long_options as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naaa. That's legacy code. I would not worry too much - we are going to delete it shortly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope 🗡️

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 17, 2022
@potiuk potiuk merged commit 7dc794a into apache:main Jun 17, 2022
@potiuk potiuk deleted the fix-instructions-on-reproducing-ci-errors branch June 17, 2022 20:08
potiuk added a commit to potiuk/airflow that referenced this pull request Jun 29, 2022
…e#24402)

The old breeze-legacy used to have a possibility of very easy
reproduction of CI failures by executing the right breeze command
that contained the commit hash of the PR being tested. This has
been broken for some time after we migrated to the new breeze,
but finally it was the time when it was needed again.

This PR brings back the capability by:

* addding --image-tag parameters for tests, shell and start-airflow
  commands
* if --image-tag is specified, then rather than building the
  image, it is pulled using the specified hash
* if --image-tag is specified, the local sources are not mounted
  to breeze when started, but the sources already embedded in the
  image are used ("skipped" set for --mount-sources).
* new "removed" command value is added to --mount-sources, it
  causes breeze command to remove the sources from the image (it
  is used when installing airflow during the tests for specified
  version (it's automatically used when --use-airflow-version
  is used).

(cherry picked from commit 7dc794a)
@ephraimbuddy ephraimbuddy added this to the Airflow 2.3.3 milestone Jun 30, 2022
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants